PYTHON-4745 - Test behavior of async task cancellation #2136
Conversation
|
|
||
| class TestAsyncCancellation(AsyncIntegrationTest): | ||
| async def test_async_cancellation_closes_connection(self): | ||
| client = await self.async_rs_or_single_client(maxPoolSize=1) |
There was a problem hiding this comment.
I think all of these tests can reuse the global test client instead of creating new ones.
| @@ -391,7 +391,7 @@ async def try_next(self) -> Optional[_DocumentType]: | |||
| if not _resumable(exc) and not exc.timeout: | |||
| await self.close() | |||
| raise | |||
| except Exception: | |||
| except BaseException: | |||
There was a problem hiding this comment.
Everywhere we catch BaseException, can you add a one line comment to explain it's intentional? Something like:
# Catch KeyboardInterrupt, CancelledError, etc. and cleanup.
| await connected(self.client) | ||
| conn = one(pool.conns) | ||
| await self.client.db.test.insert_one({"x": 1}) | ||
| self.addAsyncCleanup(self.client.db.test.drop) |
There was a problem hiding this comment.
Let's use delete_many() instead of drop() for better perf.
There was a problem hiding this comment.
Better yet, we can use setUpClass/tearDownClass to insert/drop the collection only once.
There was a problem hiding this comment.
There is no asynchronous setUpClass/tearDownClass equivalent in unittest sadly. We could create our own, something like this:
async def setup_class(self):
...
self.init_class = True
async def asyncSetup(self):
...
if not self.init_class:
await self.setup_class()
There was a problem hiding this comment.
Oh right. Let's not do that because it worth be hard/impossible to do the tearDownClass part.
| async def test_async_cancellation_closes_cursor(self): | ||
| await connected(self.client) | ||
| for _ in range(2): | ||
| await self.client.db.test.insert_one({"x": 1}) |
There was a problem hiding this comment.
Here and below, use insert_many instead of insert_one in a for-loop for better perf.
| class TestAsyncCancellation(AsyncIntegrationTest): | ||
| async def test_async_cancellation_closes_connection(self): | ||
| pool = await async_get_pool(self.client) | ||
| await connected(self.client) |
There was a problem hiding this comment.
We can remove connected from these tests for better perf. If we need to get the connection we can move this below the insert_one().
ShaneHarvey
changed the title
…ation